-
Notifications
You must be signed in to change notification settings - Fork 16
RDBC-921 Added Vector Search to Client API #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v7.1
Are you sure you want to change the base?
Conversation
12ef20d
to
2500e14
Compare
cadfe8a
to
bb6283d
Compare
|
||
// Set Corax search engine type if vector fields are present | ||
if (!vectorFieldStrings.isEmpty()) { | ||
indexDefinition.getConfiguration().setSetting("Indexing.Static.SearchEngineType", "Corax"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Constants.Configuration.Indexes.INDEXING_STATIC_SEARCH_ENGINE_TYPE
@@ -107,6 +114,10 @@ public void setReduce(String reduce) { | |||
this.reduce = reduce; | |||
} | |||
|
|||
public void setVectorOptionsStrings(Map<String, VectorFieldOptions> vectorOptionsStrings) { | |||
this.vectorFieldStrings = vectorOptionsStrings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is difference between this method and setVectorFieldStrings()
?
why we have 2 methods that are setting this.vectorFieldStrings
?
@@ -109,6 +109,8 @@ public IndexDefinition createIndexDefinition() { | |||
|
|||
if (searchEngineType != null) { | |||
_definition.getConfiguration().put(Constants.Configuration.Indexes.INDEXING_STATIC_SEARCH_ENGINE_TYPE, SharpEnum.value(searchEngineType)); | |||
} else if (_definition.getFields() != null && _definition.getFields().values().stream().anyMatch(field -> field.getVector() != null)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to set vectorFieldStrings
in the index definition as we do in c# index ?
@@ -43,9 +46,12 @@ public IndexDefinition createIndexDefinition() { | |||
indexDefinitionBuilder.setPriority(getPriority()); | |||
indexDefinitionBuilder.setState(getState()); | |||
indexDefinitionBuilder.setDeploymentMode(getDeploymentMode()); | |||
indexDefinitionBuilder.setVectorFieldStrings(vectorOptionsStrings); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how this is different from line 38 ?
@@ -3,4 +3,7 @@ | |||
import java.util.HashMap; | |||
|
|||
public class IndexConfiguration extends HashMap<String, String> { | |||
public void setSetting(String key, String value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we actually need this helper method?
int[] result = new int[outputLength]; | ||
|
||
for (int i = 0; i < length; i++) { | ||
int byteIndex = i / 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in nodejs its
const byteIndex = Math.floor(i / 8);
@@ -0,0 +1,7 @@ | |||
package net.ravendb.client.documents.session; | |||
|
|||
import java.sql.Array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we use it ? we should use equivalent of c# IEnumerable
which is probably Iterable<T>
* Interface for vector embedding field factory accessor | ||
* @param <T> The type of the field | ||
*/ | ||
public interface IVectorEmbeddingFieldFactoryAccessor<T> extends IVectorField{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we miss fieldName
?
/** | ||
* Interface for vector field | ||
*/ | ||
public interface IVectorField { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this interface is empty in both c# & nodejs
|
||
import java.util.Map; | ||
|
||
public class VectorEmbeddingFieldValueFactory implements IVectorFieldValueFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arent we missing implementation of IVectorEmbeddingFieldValueFactory
interface?
based on PR ravendb/ravendb-nodejs-client#475